Can one of the admins verify this patch? To accept patch and trigger a build add comment ".ok\W+to\W+test."

really useful patch.

Hi @jesucarr, thank you for the pull request. I am fine with allowing non-expiring auth tokens, even though I think it may be a security vulnerability.

However, I am not rather uncomfortable to enable this "feature" in all existing applications. Could you please introduce a new setting to enable this feature?

var isValid = elapsedSeconds < secondsToLive ||
  (this.constructor.settings.allowEternalTTL && secondsToLive === -1);

Note that you can already achieve the same effect by setting TTL to Number.MAX_SAFE_INTEGER or similar large number.

@raymondfeng @ritch what's your opinion on this? Am I too paranoid?

@slnode ok to test

+1 for config to turn this feature on/off

ok just pushed the new setting as proposed. I think it's a good idea. However I changed the name to allowEternalTtl so it is consistent with the camelcase naming.

@jesucarr thank you for the update. Could you please add a unit-test to check the correctness of your implementation and prevent regressions in the future?

+1 good community contribution, will try it out when its merged

It feels incongruent to have this allowEternalTtl setting under AccessToken but to have other TTL-related settings under User (ttl, maxTTL, etc.). Maybe implement this by overloading User.maxTTL? maxTTL: -1

is there any good plugin for loopback api security?

If anyone is looking at workarounds and wants an expiration date longer than 1 year -- see #353

Any news on this feature getting into production?

Can one of the admins verify this patch? To accept patch and trigger a build add comment ".ok\W+to\W+test."

+1

Can one of the admins verify this patch? To accept patch and trigger a build add comment ".ok\W+to\W+test."

+1

LGTM, @bajtos @raymondfeng @ritch Thoughts? We should get this merged if there are no objections.

@jesucarr How we can use the new option after merging this change?

@ahmed-abdulmoniem you just need to set the ttl to -1 in the options

Can someone add unit tests to verify these changes?

@jesucarr I mean in which options, options of the model or what?

@ahmed-abdulmoniem yes AccessToken options, or login options https://docs.strongloop.com/display/public/LB/Logging+in+users

It feels incongruent to have this allowEternalTtl setting under AccessToken but to have other TTL-related settings under User (ttl, maxTTL, etc.). Maybe implement this by overloading User.maxTTL? maxTTL: -1

This is a good point. Perhaps we should implement this change at the place where the AccessToken is created and we have access to User.settings, so that we can set allowEthernalTTL via User settings?

Regardless of that, we need a unit-test before we can land this change. Is there any volunteer to pick up that task?

Hi guys, do you know if this is going to be merged any time soon?

++++1

A couple things before this can be merged:

• a test verifying a token is valid when create w/ infinite TTL: -1
• split out the expression into multiple lines, with a variable describing each parts of the sub expression.

I want this code to be overly-clear so that if any of us come accross it again, its obvious what it is doing. Something like:

var created = this.created.getTime();
var elapsedSeconds = (now - created) / 1000;
var secondsToLive = this.ttl;
var allowEternalTTL = this.constructor.settings.allowEternalTTL;
var isEternalToken = secondsToLive === -1;
var isValidEternalToken = isEternalToken && allowEternalTTL;
var isValid = isValidEternalToken || (elapsedSeconds < secondsToLive);

Can one of the admins verify this patch?

However, I am not rather uncomfortable to enable this "feature" in all existing applications. Could you please introduce a new setting to enable this feature?

@bajtos agree 100%

Closing in favour of #2841.